arch/xtensa: arch_cpu_idle cleanup #64760

RanderWang · 2023-11-03T07:28:01Z

move some intel cavs platform code to intel platform folder

andyross

So, I like this very much at a level of code hygiene and platform encapsulation. So it's a +1. But I'm going to repeat my question from earlier, because I really do hope we can find an answer:

Are we absolutely, 100% sure that this is a hardware issue that only affects Intel audio DSPs?

Upstream SOF on Intel is the original source for this trick, but the way it's documented there seems to imply that it's a more general bug with Cadence IP, even though no one else seems to know about it. If that's true, then we want this to stay in the arch layer so that affected SOCs from all vendors can take advantage of it.

So... are we? If not, can we get someone at Intel to ping Cadence for a proper errata reference or something?

marc-hb

If that's true, then we want this to stay in the arch layer so that affected SOCs from all vendors can take advantage of it.

So... are we? If not, can we get someone at Intel to ping Cadence for a proper errata reference or something?

Mmmmm... shouldn't the next step be for someone OTHER than Intel to reproduce the issue first? I mean it feels at least "awkward" for User X to ask common vendor A about User Y issues.

FWIW I found Cadence support very responsive in this other, unrelated toolchain issue:

XCC RG-2017.8-linux: generated object code is affected by -g2 level (the default level) + longer source paths thesofproject/sof#7114 (comment)

marc-hb · 2023-11-06T17:40:38Z

soc/xtensa/intel_adsp/cavs/power.c

@@ -180,6 +180,52 @@ void pm_state_exit_post_ops(enum pm_state state, uint8_t substate_id)
 }
 #endif /* CONFIG_PM */

+#ifdef CONFIG_ARCH_CPU_IDLE_CUSTOM
+/* xt-clang removes any NOPs more than 8. So we need to set


Suggested change

/* xt-clang removes any NOPs more than 8. So we need to set

/* xt-clang removes any NOPs more than 8. So we need to set

Please provide at least one xt-lang version that with confirmed reproduction. This workaround should refer to a proper issue number but for now the mere output of xt-lang --version will already be infinitely better than nothing.

If there was a past discussion in https://github.com/thesofproject/sof about this then also add the link. Toolchain issues are incredibly time-consuming so the last thing they need is incomplete information.

I realize you're "only" moving an existing comment but as noted by @andyross this PR does far more than just re-organizing code.

I think @dcpleung was the one who discovered that particular optimizer misfeature, likely on the MTL toolchain?

All the xt-clang I used exhibited the same behavior of "ignoring" NOPs beyond 8.

RanderWang · 2023-11-07T01:52:41Z

So, I like this very much at a level of code hygiene and platform encapsulation. So it's a +1. But I'm going to repeat my question from earlier, because I really do hope we can find an answer:

Are we absolutely, 100% sure that this is a hardware issue that only affects Intel audio DSPs?

Upstream SOF on Intel is the original source for this trick, but the way it's documented there seems to imply that it's a more general bug with Cadence IP, even though no one else seems to know about it. If that's true, then we want this to stay in the arch layer so that affected SOCs from all vendors can take advantage of it.

So... are we? If not, can we get someone at Intel to ping Cadence for a proper errata reference or something?

For LX6, WAITI instruction has some requirements, each vendor may have different solution to meet them. Due to intel dsp clock gating method and WAITI requirement, Intel audio hw use this workaround so that no hw hang would happen. Please check internal issue about the hw detail.

updated : internal number 1804185050

nashif · 2023-11-07T12:05:53Z

arch/xtensa/Kconfig

@@ -173,4 +173,11 @@ endif # XTENSA_MMU

 endif # CPU_HAS_MMU

+config ARCH_CPU_IDLE_CUSTOM


this should not be xtensa specific, please move it up so any other architecture can do the same if needed.

@nashif updated, thanks

kv2019i

Overall I think this is a good cleanup, but I think this has to be enabled by default for affected targets.

kv2019i · 2023-11-08T11:26:29Z

arch/Kconfig

+	bool "Custom arch_cpu_idle implementation"
+	default n
+	help
+	  This options allows applications to override the default arch idle implementation with


Not sure about this. Why would this be an application level override? It seems this is a board/soc specific requirement and if you buiild for these targets, the custom functions has to be used.

@kv2019i I tested cavs2.5 and didn't find any issue. I traced this issue which was found on cavs1.8 platforms in 2017 and then these patches was applied. I don't think it still exists on cavs2.5 so I don't enable it with cavs2.5. I reserve it since It will make effect when cavs1.8 platform are supported.

arch/xtensa/core/cpu_idle.c

Each arch platform may has a general arch_cpu_idle implementation but each vendor may has a custom one, so this config will be used for vendor to override it. Some workarounds were introduced for intel cavs2.5 platform bring up. It is not general so move them to platform code. Signed-off-by: Rander Wang <rander.wang@intel.com>

Cavs platforms starts from Apllolake to Raptorlake. Some of them need some workaround for arch_cpu_idle so create a bespoken one. Each workaround is configured by kconfig setting. Signed-off-by: Rander Wang <rander.wang@intel.com>

RanderWang · 2023-11-09T06:02:06Z

remove "no optimization"

Some workarounds were introduced for intel cavs2.5 platform bring up. It is not general so move them to platform code. Signed-off-by: Rander Wang <rander.wang@intel.com>

RanderWang · 2023-11-14T05:37:33Z

@dcpleung move CONFIG_XTENSA_CPU_IDLE_SPIN and CONFIG_XTENSA_WAITI_BUG to intel side. Thanks!

RanderWang · 2023-11-17T06:52:29Z

@dcpleung could you help to review it ? thanks!

kv2019i

Thanks @RanderWang , looks good now!

RanderWang requested review from dcpleung, andyross and nashif as code owners November 3, 2023 07:28

zephyrbot added platform: Intel ADSP Intel Audio platforms area: Xtensa Xtensa Architecture labels Nov 3, 2023

zephyrbot requested review from aborisovich, ceolin, jxstelter, kv2019i, lgirdwood, lyakh, marc-hb, marcinszkudlinski, softwarecki and tmleman November 3, 2023 07:28

zephyrbot assigned dcpleung Nov 3, 2023

andyross previously approved these changes Nov 6, 2023

View reviewed changes

marc-hb reviewed Nov 6, 2023

View reviewed changes

tmleman previously approved these changes Nov 7, 2023

View reviewed changes

nashif reviewed Nov 7, 2023

View reviewed changes

tmleman mentioned this pull request Nov 7, 2023

intel_adsp: ace: clock gating in idle #64468

Merged

RanderWang dismissed stale reviews from tmleman and andyross via cf5b1f8 November 8, 2023 02:12

RanderWang force-pushed the cpu_idle_cleanup branch from d2a9535 to cf5b1f8 Compare November 8, 2023 02:12

kv2019i requested changes Nov 8, 2023

View reviewed changes

dcpleung requested changes Nov 8, 2023

View reviewed changes

arch/xtensa/core/cpu_idle.c Show resolved Hide resolved

arch/xtensa/core/cpu_idle.c Outdated Show resolved Hide resolved

RanderWang added 2 commits November 9, 2023 13:54

soc: intel_adsp/cavs: add arch_cpu_idle support

4bea568

Cavs platforms starts from Apllolake to Raptorlake. Some of them need some workaround for arch_cpu_idle so create a bespoken one. Each workaround is configured by kconfig setting. Signed-off-by: Rander Wang <rander.wang@intel.com>

RanderWang force-pushed the cpu_idle_cleanup branch from cf5b1f8 to 1a3f305 Compare November 9, 2023 06:01

arch/xtensa: clean up arch_cpu_idle function

0b1b664

Some workarounds were introduced for intel cavs2.5 platform bring up. It is not general so move them to platform code. Signed-off-by: Rander Wang <rander.wang@intel.com>

RanderWang force-pushed the cpu_idle_cleanup branch from 1a3f305 to 0b1b664 Compare November 14, 2023 05:36

dcpleung approved these changes Nov 14, 2023

View reviewed changes

RanderWang requested a review from nashif November 17, 2023 06:52

dcpleung approved these changes Nov 17, 2023

View reviewed changes

kv2019i approved these changes Nov 20, 2023

View reviewed changes

carlescufi merged commit 9549012 into zephyrproject-rtos:main Nov 20, 2023
24 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

arch/xtensa: arch_cpu_idle cleanup #64760

arch/xtensa: arch_cpu_idle cleanup #64760

RanderWang commented Nov 3, 2023

andyross left a comment

marc-hb left a comment

marc-hb Nov 6, 2023

andyross Nov 6, 2023

dcpleung Nov 7, 2023

RanderWang commented Nov 7, 2023 •

edited

Loading

nashif Nov 7, 2023

RanderWang Nov 8, 2023

kv2019i left a comment

kv2019i Nov 8, 2023

RanderWang Nov 9, 2023

RanderWang commented Nov 9, 2023

RanderWang commented Nov 14, 2023

RanderWang commented Nov 17, 2023

kv2019i left a comment

	/* xt-clang removes any NOPs more than 8. So we need to set
	/* xt-clang removes any NOPs more than 8. So we need to set

		@@ -173,4 +173,11 @@ endif # XTENSA_MMU

		endif # CPU_HAS_MMU

		config ARCH_CPU_IDLE_CUSTOM

arch/xtensa: arch_cpu_idle cleanup #64760

arch/xtensa: arch_cpu_idle cleanup #64760

Conversation

RanderWang commented Nov 3, 2023

andyross left a comment

Choose a reason for hiding this comment

marc-hb left a comment

Choose a reason for hiding this comment

marc-hb Nov 6, 2023

Choose a reason for hiding this comment

andyross Nov 6, 2023

Choose a reason for hiding this comment

dcpleung Nov 7, 2023

Choose a reason for hiding this comment

RanderWang commented Nov 7, 2023 • edited Loading

nashif Nov 7, 2023

Choose a reason for hiding this comment

RanderWang Nov 8, 2023

Choose a reason for hiding this comment

kv2019i left a comment

Choose a reason for hiding this comment

kv2019i Nov 8, 2023

Choose a reason for hiding this comment

RanderWang Nov 9, 2023

Choose a reason for hiding this comment

RanderWang commented Nov 9, 2023

RanderWang commented Nov 14, 2023

RanderWang commented Nov 17, 2023

kv2019i left a comment

Choose a reason for hiding this comment

RanderWang commented Nov 7, 2023 •

edited

Loading